Skip to content

Add renderer id to react-devtools injection#10475

Merged
gaearon merged 3 commits into
react:masterfrom
chisler:detect-duplicated-react
Aug 21, 2017
Merged

Add renderer id to react-devtools injection#10475
gaearon merged 3 commits into
react:masterfrom
chisler:detect-duplicated-react

Conversation

@chisler

@chisler chisler commented Aug 17, 2017

Copy link
Copy Markdown
Contributor

Helps to resolve react-devtools 700 — reporting multiple react-dom copies.

I added id argument to detect react-dom without code-guessing.

@gaearon

gaearon commented Aug 17, 2017

Copy link
Copy Markdown
Collaborator

I think this is good, thanks.

@sebmarkbage

Copy link
Copy Markdown
Contributor

@gaearon Should we bikeshed the name a bit since we have to live with it forever?

// This is an enum because we may add more (e.g. profiler build)
bundleType: __DEV__ ? 1 : 0,
version: ReactVersion,
id: 'react-dom',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rendererType: 'react-dom'?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rendererName? rendererUniqueName? People cloning ReactDOM might think a "type" would still be ReactDOM.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rendererPackageName? As in the npm name.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me!

@gaearon gaearon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change to rendererPackageName.

@chisler

chisler commented Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

Hey! I changed the name.

bundleType: __DEV__ ? 1 : 0,
version: ReactVersion,
id: 'react-dom',
rendererPackageName: 'react-dom',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also please add this to React Native entries?

@chisler

chisler commented Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

It seems like we don't want to see any renderer duplicated, not react-dom only. But that might just not exist in the real world.

About adding to react-art/....

@gaearon

gaearon commented Aug 21, 2017

Copy link
Copy Markdown
Collaborator

Can you file another PR for ReactART? I think it doesn't even contain a call now. We should add it.

@gaearon gaearon merged commit ec77740 into react:master Aug 21, 2017
@chisler

chisler commented Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

I'll do that. I'll update the devtools PR as well, detecting not react-dom only, but two duplicated renderers that provided id or were detected by code.

@gaearon

gaearon commented Aug 21, 2017

Copy link
Copy Markdown
Collaborator

That sounds great. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants